Skip to content

Add experimental least-load strategy, proxy-maintained inflight counters, and instance-load debug API#118

Open
zhy1658858023 wants to merge 3 commits into
mainfrom
codex/add-least_load-instance-selection-strategy
Open

Add experimental least-load strategy, proxy-maintained inflight counters, and instance-load debug API#118
zhy1658858023 wants to merge 3 commits into
mainfrom
codex/add-least_load-instance-selection-strategy

Conversation

@zhy1658858023

Copy link
Copy Markdown
Collaborator

Motivation

  • Provide an experimental load-aware Instance selection policy that prefers the lowest-known runtime load and falls back to round-robin when metrics are unavailable.
  • Maintain a Proxy-local inflight lifecycle counter so selection can rely on a consistent, local primary load signal rather than heartbeats alone.
  • Expose per-instance load debug information and document the new strategy and usage in the Proxy README.

Description

  • Added a new least-load strategy implementation in proxy/strategy/least_load.py that prefers load.inflight then load.qps_1m, and falls back to round-robin using RoundRobinStrategy when no metrics exist.
  • Updated proxy/strategy/factory.py to register the least_load strategy aliases (ll, least-load).
  • Modified proxy/resource/instance_pool.py to stop treating inflight as heartbeat-maintained, added begin_request and end_request methods to increment/decrement the Proxy-maintained inflight counter, and implemented snapshot_instance_loads to return load/debug info (including queue hints) used by strategies and debug views.
  • Updated proxy/proxy.py to call begin_request immediately after selecting an instance and to call end_request at the end of request lifetime in both streaming and non-streaming paths, and to guard against the selected instance being unregistered by returning 503 if begin_request fails.
  • Introduced the /debug/instance_loads control-plane endpoint in proxy/resource/p_control_plane.py that queries the pool snapshot_instance_loads and optionally gathers queue-depth hints from the registered queue snapshot provider.
  • Adjusted resource metrics metadata to reflect the new proxy_lifecycle_counter source for inflight totals and added README documentation and CLI notes to document least_load selection and debug endpoints.

Testing

  • No automated tests were executed as part of this change.

Codex Task

@zhy1658858023 zhy1658858023 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我不建议直接合并当前 PR #118,需要先清理分支并修一个策略语义问题。

主要问题:

  1. 分支不干净 / 包含 PR #116 内容

当前 PR 的 head 仍然是 codex/add-least_load-instance-selection-strategy,也就是 PR #116 用过的分支。compare 显示当前分支相对 maindivergedbehind_by=3,merge base 仍然是 PR #116 合并前的提交。因此 proxy/strategy/least_load.pyproxy/strategy/factory.pyproxy/README.md 里的 PR #116 内容又被重复包含进了 PR #118

请从最新 origin/main 新建干净分支,例如:

codex/issue-117-proxy-instance-load-counters

然后只提交 Issue #117 的增量。PR diff 应该主要集中在:

proxy/resource/instance_pool.py
proxy/proxy.py
proxy/resource/p_control_plane.py
proxy/README.md  # 如只补充 debug/load-counter 文档

除非确实需要调整策略,否则不要重新提交 PR #116 已经合并的 least_load.py / factory.py 内容。

  1. Proxy-maintained inflight 应该初始化所有 alive Instance,而不是只在 begin_request 后才变成 known

当前 begin_request() 会把 None 当作 0 并加 1,end_request() 会降到 0。这意味着被选中过的实例会变成已知 inflight=0,但从未被选中过的新实例仍然是 inflight=None

least_load 当前会在存在任何 known inflight 的情况下,只在 known candidates 里选择。这样可能导致:

instance A 被选中过 -> inflight 变成已知 0
instance B 从未被选中过 -> inflight 仍是 unknown
least_load 后续只看 known inflight,可能一直选择 A,B 被饥饿

建议在 InstancePool.upsert() 创建新 InstanceInfo 时,把 Proxy-maintained load.inflight 初始化为 0。这样在 Proxy 已经接管 lifecycle counter 后,所有 alive Instance 都有真实的本地 inflight 值,least_load 不会把 unknown 和 known 混在一起造成偏置。

保留的好方向:

  • begin_request / end_request 集中在 InstancePool 内部是对的。
  • chat streaming 将 decrement 放进 streaming generator lifecycle 是对的。
  • completions 用 finally decrement 是对的。
  • /debug/instance_loads 的方向是对的。
  • pool_resource.metric_source.inflight_total = proxy_lifecycle_counter 是合理的。

请修正后再提交一个干净 PR。PR 描述里请明确:

  • base 是最新 origin/main
  • 不包含 PR #116 的重复 diff
  • changed files 仅限 Issue #117 所需文件
  • inflight 初始化策略如何避免 unknown-instance 饥饿
  • 已运行 py_compile 和 begin/end counter smoke test

@zhy1658858023 zhy1658858023 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我看了更新后的 PR #118,这版有进展,但仍然不建议直接合并。

已经修好的点:

  • InstancePool.upsert() 现在会在新建 InstanceInfo 时设置 load=InstanceLoad(inflight=0),已有 Instance 如果 load.inflight is None 也会补成 0。这修掉了上轮指出的 unknown Instance 饥饿问题。
  • begin_request() / end_request() 集中在 InstancePool 内部维护 inflight,并且 end_request() 有 floor 0。
  • heartbeat 不再覆盖 inflight,只保留 qps_1m / gpu_util 等非 lifecycle load 字段。
  • chat streaming 和 completions 都接入了 begin/end 生命周期钩子。
  • /debug/instance_loads 的方向正确。

但 PR 仍有一个关键问题:它仍然包含 PR #116 的重复 diff

当前 PR #118 的文件列表仍包含:

proxy/strategy/least_load.py
proxy/strategy/factory.py
proxy/README.md

其中 proxy/strategy/least_load.pyproxy/strategy/factory.py 是 PR #116 已经实现并合并过的内容,不应该再次作为 Issue #117 的增量出现。README 里也混合了 PR #116 的策略文档和 Issue #117/debug/instance_loads 文档,导致 review 边界不清楚。

请继续清理分支,目标是让 PR #118 只包含 Issue #117 的增量。理想 changed files 应该主要是:

proxy/resource/instance_pool.py
proxy/proxy.py
proxy/resource/p_control_plane.py
proxy/README.md   # 只补充 inflight lifecycle / debug endpoint 文档

除非确实要微调 least_load 策略本身,否则不要让 proxy/strategy/least_load.pyproxy/strategy/factory.py 出现在 PR #118 的 diff 中。

建议做法:

  1. 从最新 origin/main 新开干净分支:
codex/issue-117-proxy-instance-load-counters
  1. 只 cherry-pick / 重新应用 Issue #117 的增量:

    • InstancePool.begin_request/end_request/snapshot_instance_loads
    • load.inflight=0 初始化
    • proxy.py 请求生命周期 begin/end hook
    • /debug/instance_loads
    • README 只补充 load counter/debug endpoint 的说明
  2. PR 描述里明确:

    • 不包含 PR #116 的重复 diff
    • changed files 只对应 Issue #117
    • least_load.py / factory.py 如无变化不应出现在 diff 中
    • 已运行 py_compile 和 begin/end counter smoke test

功能方向已经对了,现在主要是 PR 边界需要继续清理。

@zhy1658858023 zhy1658858023 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这一版比上一版干净很多,主要问题已经修掉了。

检查结果:

  • PR 现在 mergeable=true
  • changed files 已经从 6 个降到 4 个:
    • proxy/resource/instance_pool.py
    • proxy/proxy.py
    • proxy/resource/p_control_plane.py
    • proxy/README.md
  • proxy/strategy/least_load.pyproxy/strategy/factory.py 已经不再出现在 diff 里,PR #116 的重复策略 diff 已经被清掉。
  • InstancePool.upsert() 新建 Instance 时初始化 load=InstanceLoad(inflight=0),已有 Instance 若 load.inflight is None 也会补为 0,解决了 unknown Instance 饥饿问题。
  • heartbeat() 不再覆盖 inflight,符合 Proxy lifecycle counter 接管 inflight 的设计。
  • begin_request() / end_request() 集中在 InstancePool,并且 end_request() 有 floor 0。
  • chat streaming 的 decrement 放在 generator lifecycle 的 finally 里;completions 也用 finally 做 decrement。
  • /debug/instance_loads 已接入,并能带 queue snapshot provider 的 per-instance queue hints。

剩余注意点:

  • PR metadata 里的 body 仍然有一些旧描述,例如“Added least-load strategy implementation / Updated factory aliases”,这已经不是当前 diff 的内容了。建议更新 PR description,避免 review 时误解。
  • Testing 仍写着没有自动测试。合并前建议至少在本地跑 py_compile、begin/end counter smoke test、以及 /debug/instance_loads runtime smoke test。
  • compare 仍显示分支历史是 diverged,但当前文件 diff 已经是 Issue #117 范围,且 GitHub 显示 mergeable=true。若 GitHub 页面没有强制要求 update branch,这不是 blocker。

我建议:本地 smoke test 通过后可以合并。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant